Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Look for javac instead of java #184

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Look for javac instead of java #184

wants to merge 2 commits into from
+13 −1

Conversation

arlaneenalra
Copy link

There are some situations where using bin/java as a heuristic to find JAVA_HOME within a JDK package can result in mistakenly finding a JRE root instead. Looking for bin/javac fixes this since JREs do not have javac.

This should fix #97

Before this PR

The findJavaHome method could mistakenly find a JRE inside of the JDK packages instead of the JDK resulting in an unusable JAVA_HOME. This main impacted some older JDKs.

After this PR

==COMMIT_MSG==
Use bin/javac to find JAVA_HOME instead of bin/java to avoid accidentally installing a JRE instead of a JDK.
==COMMIT_MSG==

Possible downsides?

@arlaneenalra
Look for javac instead of java
82f6f58 
There are some situations where using `bin/java` as a heuristic
to find JAVA_HOME within a JDK package can result in mistakenly
finding a JRE root instead. Looking for `bin/javac` fixes this
since JREs do not have `javac`.

This should fix palantir#97
@palantirtech
Copy link
Member

Thanks for your interest in palantir/gradle-jdks, @arlaneenalra! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@changelog-app
Copy link

changelog-app bot commented Apr 2, 2023

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Use bin/javac to find JAVA_HOME instead of bin/java to avoid accidentally installing a JRE instead of a JDK.

Check the box to generate changelog(s)

  • Generate changelog entry

@arlaneenalra
Copy link
Author

Note: I'll sign the CLA .. but the rest of those items seem a bit much ..

@arlaneenalra
Copy link
Author

Bump: What is needed to get this reviewed?

@jonahbeckford
Copy link

My error is different from the summary's linked issue #97, but I think this PR would fix the wrong selection of the JRE rather than the JDK when I'm running into with Java 8:

> Task :ffi-java:compileJava FAILED
Compiling with JDK Java compiler API.
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':ffi-java:compileJava'.
> Could not find tools.jar. Please check that /root/.gradle/gradle-jdks/azul-zulu-8.72.0.17-8.0.382-569852f9b038db1a contains a valid JDK installation.

@arlaneenalra: It looks like you have to press two checkmark boxes ("Type: Fix" and "Check the box to generate changelog(s)"). Is that possible?

@arlaneenalra
Copy link
Author

@arlaneenalra: It looks like you have to press two checkmark boxes ("Type: Fix" and "Check the box to generate changelog(s)"). Is that possible?

Maybe .. I have no idea how that changelog-app thing works and can't find the documentation in my cursory glance online .. I'll take a crack at adding what I think might be the correct file ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support using the plugin with Java 8
3 participants
@arlaneenalra @palantirtech @jonahbeckford